Skip to content

Format remote configuration values consistently with local settings - #999

Open
the8thbit wants to merge 1 commit into
meshtastic:masterfrom
the8thbit:fix/remote-key-format-956
Open

Format remote configuration values consistently with local settings#999
the8thbit wants to merge 1 commit into
meshtastic:masterfrom
the8thbit:fix/remote-key-format-956

Conversation

@the8thbit

@the8thbit the8thbit commented Oct 23, 2026 •
edited by coderabbitai Bot
Loading

Copy link
Copy Markdown

Remote configuration responses now print setting values with the same formatting as local --get output. Singular and repeated byte fields use base64: values that can be reused with --set, including security.admin_key, instead of protobuf octal escapes. Received configuration is still copied into the node cache before printing.

Fixes #956.

Validation: real protobuf response tests compare remote security and module settings against local output, cover repeated and empty byte values, and verify caching and ACK/NAK handling. Default pytest: 488 passed; pylint: 10.00/10. Mypy reports the same existing Listener/Callable type error in meshtastic/slog/slog.py:183 on this branch and unchanged master (84 source files checked).

Summary by CodeRabbit

  • Bug Fixes
    • Remote settings responses now display configuration values field by field rather than as a single configuration object. Repeated values are shown individually. This applies to security and module settings, making remote responses easier to read and compare with locally displayed settings.

@coderabbitai

coderabbitai Bot commented Oct 23, 2026 •
edited
Loading

Copy link
Copy Markdown

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
📥 Commits

Reviewing files that changed from the base of the PR and between 0a18357 and 2625b4a.

📒 Files selected for processing: 2
  • meshtastic/node.py
  • meshtastic/tests/test_node.py

Included review availability: This review used your included allowance. Your plan provides up to 4 included reviews per hour; 2 remain after this review.

📝 Walkthrough

The remote settings response now prints configuration fields individually, including repeated values formatted with toStr. Tests cover security and MQTT settings, configuration state, acknowledgment flags, and a rejected request.

Changes: Remote settings response

Layer / File(s) Summary
Format settings output and validate responses
meshtastic/node.py, meshtastic/tests/test_node.py		 The response prints each configuration field with its type and field name. Repeated values are formatted element by element. Tests check security and MQTT output, configuration state, and the NOT_AUTHORIZED response.

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~10 minutes

Change: Bug fix · Severity of issue fixed: Low

Suggested reviewers: mverch67

Merge Risk: ⚪ Minimal · up to 2625b

Remote settings output is consistent with local output for the populated values this change targets. No issue identified here needs to block merging.

Pre-merge checks: Passed 5
Check name Status Explanation
Description Check Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check Passed The title clearly and concisely describes the main change: consistent formatting of remote configuration values with local settings output.
Linked Issues check Passed Issue [#956] requires consistent --get security.admin_key formatting for direct and remote responses. In Node.onResponseRequestSettings, the PR copies the raw remote protobuf into the node cache, …
Out of Scope Changes check Passed The reviewed changes are limited to remote configuration response formatting in meshtastic/node.py and tests for those responses in meshtastic/tests/test_node.py. The import, field-formatting logi…
Docstring Coverage Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 5 functions across 2 files.
Full details: Linked Issues check

Explanation

Issue [#956] requires consistent --get security.admin_key formatting for direct and remote responses. In Node.onResponseRequestSettings, the PR copies the raw remote protobuf into the node cache, then formats each returned field with toStr; byte fields therefore use reusable base64: values, including repeated security.admin_key values. The supplied test summary covers security and module responses, repeated and empty byte values, local-output comparisons, caching, and ACK/NAK handling.

Full details: Out of Scope Changes check

Explanation

The reviewed changes are limited to remote configuration response formatting in meshtastic/node.py and tests for those responses in meshtastic/tests/test_node.py. The import, field-formatting logic, cache update, and response tests directly support issue [#956]. No unrelated change is shown.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Bug: --get security.adminKey responds in different formats to LoRa (--dest) and WiFi (--tcp)

1 participant

@the8thbit